custom client option #89
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR adds a CustomClient field to the PushOptions struct to enable users to specify a custom HTTP client for metric pushing. This addresses the need for mutual TLS authentication with client certificates when pushing metrics to VictoriaMetrics instances without modifying the global HTTP DefaultTransport.
- Added
CustomClient *http.Clientfield toPushOptionsstruct for custom HTTP client configuration - Modified the client creation logic in
newPushContextto use the provided custom client or fall back to default - Added comprehensive test coverage to verify custom client functionality with TLS configurations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| push.go | Adds CustomClient field to PushOptions and modifies newPushContext to use custom or default client |
| push_test.go | Adds test coverage for custom client functionality with TLS configuration scenarios |
Comments suppressed due to low confidence (1)
push_test.go:290
- The test relies on string matching to verify TLS-related errors, which is brittle. Consider using error type assertions or wrapping errors to make the test more robust and specific.
// Verify the error is TLS-related
| } | ||
|
|
||
| func TestCustomClientWithTLSConfig(t *testing.T) { | ||
| // Create a TLS configuration that accepts self-signed cert |
There was a problem hiding this comment.
[nitpick] The comment should use 'certificates' instead of 'cert' for proper grammar.
Suggested change
| // Create a TLS configuration that accepts self-signed cert | |
| // Create a TLS configuration that accepts self-signed certificate |
f41gh7
reviewed
Jul 27, 2025
| // CustomClient allows specifying a custom HTTP client for making the push requests. | ||
| // | ||
| // If nil, the default http.Client will be created. | ||
| CustomClient *http.Client |
There was a problem hiding this comment.
I think, it's more clear to name it just as Client and add a comment, that it's optional http client for making push requests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add CustomClient option to PushOptions
Problem
When pushing metrics to a VictoriaMetrics Instance that requires mutual TLS authentication with client certificates, the current implementation doesn't allow customizing the HTTP client used for requests. This creates a dilemma:
Solution
Add a
CustomClientfield to thePushOptionsstruct that allows specifying a custom HTTP client. This enables users to configure TLS settings specifically for metrics pushing without affecting other parts of the application.Why Not Use vmauth or Traefik?
While vmauth and Traefik are excellent solutions for many scenarios, there are specific use cases where a custom client approach provides significant advantages:
Cross-cloud deployments: When the metrics client is in one cloud provider and the VictoriaMetrics server is in another, setting up an additional proxy layer introduces unnecessary complexity and potential points of failure.
Network complexity: Cross-cloud communications typically traverse public internet, making embedded TLS configuration simpler and more secure than exposing and securing additional proxy services.
Operational simplicity: For many users, configuring the client directly eliminates the need to deploy, manage, and secure additional infrastructure components.
This implementation provides flexibility for users to choose the approach that best fits their specific infrastructure and security requirements.
Use Case
This is particularly useful in multi-cloud environments where metrics need to be pushed between services in different clouds requiring mutual TLS authentication. For example:
Changes
CustomClient *http.Clientfield toPushOptionsstructnewPushContextto use the provided client or create a default oneThe implementation maintains backward compatibility with all existing code.
Fixes #88